Skip to content

fix: suggested fixes for #3514 #3538

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

NoelStephensUnity
Copy link
Collaborator

Suggested fixes for the discovered DA only issue where non-authority instances would not automatically remove children from a parent NetworkObject being despawned by the authority.

See: #3514

Changelog

NA

Testing and Documentation

  • Includes minor integration test modifications.
  • No documentation changes or additions were necessary.

Backport

No backport needed as this is all v2.x.

michalChrobot and others added 27 commits June 24, 2025 10:50
This PR bumps Wrench to the latest version and regnerates recipes (I
will close automated update PRs after). This update allows us to modify
timeouts and add additional project dependencies for the validation jobs
so I added **testproject** validation as dependency for our releases (I
didn't add minimalproject since there are no tests and
testproject-tools-integration since this will be moved to Tools repo).

Beside adding testproject as the default project for running EditMode
and PlayMode tests I also permanently bumped validation job timeout to
40m (option available from this Wrench version) since in the past we
needed to bump it manually from 20m since sometimes our jobs were timing
up

I tested the change and now we are running all package and project tests
correctly when releasing with Wrench scripts

## Backport

#3513
[MTTB-950](https://jira.unity3d.com/browse/MTTB-950)

implements: #2668 
fixes: #2359 

## Changelog

- Added: Mappings for ClientId to TransportId

## Testing and Documentation

- Includes unit tests.

## Backport

Backported in #3515

---------

Co-authored-by: Patrick Coglan <[email protected]>
Fixing issue with missing using directive.
Updating codeowners file for 2.x so I can stalk the documentation
folder.

## Backport

#3524
Chore to merge the hot-fix patch v2.4.3 back into the develop-2.0.0
branch.


## Changelog

NA

## Testing and Documentation

- No tests have been added.
- No documentation changes or additions were necessary.

<!-- Uncomment and mark items off with a * if this PR deprecates any
API:
### Deprecated API
- [ ] An `[Obsolete]` attribute was added along with a `(RemovedAfter
yyyy-mm-dd)` entry.
- [ ] An [api updater] was added.
- [ ] Deprecation of the API is explained in the CHANGELOG.
- [ ] The users can understand why this API was removed and what they
should use instead.
-->

## Backport

This is a v2.x specific PR.

<!-- If this is a backport:
 - Add the following to the PR title: "\[Backport\] ..." .
 - Link to the original PR.
If this needs a backport - state this here
If a backport is not needed please provide the reason why.
If the "Backports" section is not present it will lead to a CI test
failure.
-->

---------

Co-authored-by: unity-renovate[bot] <120015202+unity-renovate[bot]@users.noreply.github.com>
Co-authored-by: Emma <[email protected]>
Recently CodeCov was improved to also work for GitHub.com repositories
so we can finally update the configuration to upload our results.
This PR should be merged after we will be done with ongoing release
process because it may require some corrections in the way how CodeCov
comments will work so I don't want to block any ongoing work

## Backport
This is ported to `develop` branch in
#3476.
Note that it will not affect `develop-2.0.0` branch since the
configuration is being taken from default branch (currently `develop`)
but when in the future we will switch the default branch then the
configuration will already be there

---------

Co-authored-by: Emma <[email protected]>
This PR focuses on investigating often timeouts of WebGL job happening
on Ubuntu platform (note that there are no timeouts on Windows) and also
on re-enabling macOS job that was disabled due to error "_Light baking
could not be started because no valid OpenCL device could be found_" and
is tracked in MTT-11726

When it comes to the error on macOS, the build was failing due to a
light baking error in different scenes with message "_no valid OpenCL
device could be found._" This common issue on Apple Silicon VMs, where
OpenCL support is often missing for the GPU lightmapper, can be resolved
by disabling "Baked Global Illumination" for the scene. Since those
scenes in testproject do not really need baked lighting and I didn't
spot any difference with this option disabled I went forward with
disabling it for testproject scenes and this resolves macOS issue.
**Note** that most changes are related to testproject updates that
happened automatically when I changed the mentioned lighting setting

For the Ubuntu timeout problem I restored image version to v4 instead of
pointing to specific version since the issue that caused us to do so was
resolved. I also increased flavor to xlarge (from large) because it
seems that with more resources the process is not hanging anymore (will
run test 5 times to confirm)

## Backport

#3511
…(develop-2.0.0) (#3532)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| RecipeEngine.Modules.Wrench | nuget | patch | `0.12.1` -> `0.12.2` |

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Unity Renovate
Bot](https://internaldocs.unity.com/renovate/).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIwLjAuMC1zZW1hbnRpYy1yZWxlYXNlIiwidXBkYXRlZEluVmVyIjoiMC4wLjAtc2VtYW50aWMtcmVsZWFzZSIsInRhcmdldEJyYW5jaCI6ImRldmVsb3AtMi4wLjAiLCJsYWJlbHMiOltdfQ==-->

## Backport

#3531

Co-authored-by: unity-renovate[bot] <120015202+unity-renovate[bot]@users.noreply.github.com>
Adding documentation to the 2.4 version of the package ahead of
deprecating the Docusaurus-based site.

## Changelog

- Added: Package documentation.

## Testing and Documentation

- Includes edits to existing public manual documentation.
- Tested and building locally.

## Backport

No plan to backport to older 2.x versions, too much faff involved in the
conversion from Docusaurus format to standard package Markdown format.
(corresponding PR is
#3520)
removing using directive.
Fixing issue with missing using directive.
fixing some of the DA specific despawn issues.
@NoelStephensUnity NoelStephensUnity requested a review from a team as a code owner July 8, 2025 23:02
updated one of the comments
Adjusting the hasDAAuthority to include DAHost.
we should be able to create a PR from another fix PR and have CI trigger on that too.
@NoelStephensUnity NoelStephensUnity requested review from EmandM and a team as code owners July 9, 2025 00:05
Removing single white space after comment.
@@ -66,6 +66,7 @@ pull_request_trigger:
- "develop"
- "develop-2.0.0"
- "/release\/.*/"
- "/fix\/.*/"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why to add this automatically? My understanding is that we don't need to "waste resources" and enforce the test passing for PRs that are not "landing". So what I would do is to trigger those tests manually in Yamato if you want to ensure that it's passing.
Otherwise notice that if there is a PR targeting develop branch to which you are merging then after the merge the tests will automatically run on that branch

Also fix/ may be a bit general in a way that someone may be surprised by enforced tests (in my opinion)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why to add this automatically? My understanding is that we don't need to "waste resources" and enforce the test passing for PRs that are not "landing". So what I would do is to trigger those tests manually in Yamato if you want to ensure that it's passing.
Otherwise notice that if there is a PR targeting develop branch to which you are merging then after the merge the tests will automatically run on that branch

Adding /fix/. to the triggers should have zero impact on triggering anything on the develop branch.

Also fix/ may be a bit general in a way that someone may be surprised by enforced tests (in my opinion)

For fixes, using "fix/" as the prefix to a branch name is actually the standard.... fix/
Just like for chores, using "chore" as the prefix to a branch name is actually the standard.... chore/

But all of this is just my recommended updates/fixes and we can just as easily close the PR and delete the branch.

Copy link
Collaborator

@michalChrobot michalChrobot Jul 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding /fix/. to the triggers should have zero impact on triggering anything on the develop branch.

Maybe I actually described it a bit wrongly, but my point is just that this job will be triggered twice. Since the trigger definition will be

    pull_requests:
      - targets:
          only:
            - "develop"
            - "develop-2.0.0"
            - "/release\/.*/"
            - - "/fix\/.*/"

This mean that this entire set of jobs will run on ANY PR that targets fix/... branch. Assumption is that said fix/ branch will either target develop/develop-2.0.0 (in that case this set of jobs will run as per job trigger definition) or it will merge to some other branch that at some point in the future will target develop/develop-2.0.0 (since otherwise it will never be part of the package) and in that point this set of jobs will run

So that's why I'm more into "If your branch is not targeting develop/develop-2.0.0 then we don't force running those jobs in order for the PR to merge"

@EmandM EmandM force-pushed the fix/destroy-in-scene-placed-objects branch from 41fc2c8 to ba1eb00 Compare July 10, 2025 15:40
@NoelStephensUnity
Copy link
Collaborator Author

Closing pull request since issues were resolved in #3514.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants